Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Conversation

@yannicks1
Copy link
Collaborator

@yannicks1 yannicks1 commented Feb 20, 2025

Ensuring compatibility with the latest upstream release v.0.7.3. Since there has been a new abstract method introduced in the base model runner, we have to define it in the Spyre model runner which inherits from the base class.

changes:

  • adding get_model() function in SpyreModelRunner as required by abstract base class ModelRunnerBase (link to upstream base class abstract method)
  • syncing vllm-spyre/core/scheuduler.py with the upstream scheduler v.0.7.3.
  • Adding comments # SPYRE SPECIFIC CODE BLOCK START and # SPYRE SPECIFIC CODE BLOCK END to mark the Spyre specific code blocks.
  • changed docker file to install vllm v0.7.3 from pip wheel.
  • created symlinks for python->python${PYTHON_VERSION} and pip->pip${PYTHON_VERSION} to only have the python version hard coded in the docker file: ARG PYTHON_VERSION=3.12

Tested vllm-spyre plugin with release v.0.7.3 (commit) on Spyre successfully.

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

  • Add ready label to the PR
  • Enable auto-merge.

🚀

Signed-off-by: Yannick Schnider <[email protected]>
Signed-off-by: Yannick Schnider <[email protected]>
@yannicks1 yannicks1 force-pushed the ysc-fix-upstream-compatibility branch 2 times, most recently from 26f24c9 to c75903e Compare February 21, 2025 10:46
Dockerfile.spyre Outdated
&& pip install -r requirements-build.txt \
&& pip install --no-build-isolation -v -e . \
&& mkdir /workspace/vllm-spyre
RUN pip${PYTHON_VERSION} install vllm==0.7.3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this is why spyre tests are still failing

Suggested change
RUN pip${PYTHON_VERSION} install vllm==0.7.3
RUN pip${PYTHON_VERSION} install --upgrade pip && pip install vllm==0.7.3

Copy link
Member

@sducouedic sducouedic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the python version! This looks good to me.

@yannicks1 yannicks1 force-pushed the ysc-fix-upstream-compatibility branch from cc7dd6f to ac12781 Compare February 21, 2025 11:28
Copy link
Member

@tdoublep tdoublep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yannicks1 yannicks1 merged commit a2d9f51 into main Feb 21, 2025
10 of 11 checks passed
@yannicks1 yannicks1 deleted the ysc-fix-upstream-compatibility branch February 21, 2025 13:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants